Current coverage is 80.29% (diff: 100%)

No coverage report found for master at 585c19f.

Powered by Codecov. Last update 585c19f...693fa7a

Okay @suhaboncukcu, first of all thank you very much for your contribution! It feels good that people like it and are motivated to improve it.

However, I do have some feedback:

• You used 5 commits to reach this PR; while you could have done it in less I guess (according to the white lines ;)). Try to minimize it.
• I like the configure stuff, but can't we add some tests to be sure default or added templates aren't overruled?
• Templates can already be added through the Configure class, right? Shouldn't we document that? *just a question

Thanks for the comments @bobmulder!

1 - I'm used to commit every change I create and didn't think that the files I didn't touch had some errors :) How should I proceed? Should I close this one and create another PR with another fork? (I should have worked with another branch indeed though, my bad!)

2 - I think, if one uses Configure class, it can always be overruled. Could you give an example for this?

3 - Could be. If there is just one template, I guess one could want to define the template on bootstrap in Configure class and go on. I'm not sure.

1 - It's okay for now, lets keep this PR :)

2 - Example could be; what if I added some templates via my Configure class (using config.php), and later on load an external config file. What will happen, how will they be merged? By adding tests, we just will be sure this works :)

3 - To be complete we would be good to add it to the docs I think. Agree?

👍

I'm sorry for my dumbness on this; tests seem to be my weak spot. How can I do that? Would you mind getting in touch on slack/gitter/skype/ etc?

I can update the docs but then again, shouldn't this be in the "black box" for the plugin? I think developers using the plugin shouldn't register the templates themselves via Configure class. There could be an option to -just for an instance- encrypt the templates via a function within the plugin before adding them to Configure. Or let's say we've added an option to be configured to get templates as json objects instead of texts. I think developers should be discouraged to use Configure class to register templates. It's just my opinion though I can add the change in this PR if you are sure about adding Configure description to docs. :)